From a22292f2bb59bda113d820fa7da5629408fac98d Mon Sep 17 00:00:00 2001 From: "emellor@leeni.uk.xensource.com" Date: Tue, 22 Nov 2005 16:31:16 +0100 Subject: [PATCH] Make it possible to run valgrind on code linked with the libxc libraries. Valgrind cannot see when a hypercall has initialised entries in a data structure, so appropriate memsets have been placed before using dom0_op_t, privcmd_hypercall_t, and a couple of miscellaneous blocks passed into this layer. This initialises the block so that valgrind considers it to be valid, but the data therein will be immediately overwritten by the hypercall, all being well. These changes are semantically neutral if -DVALGRIND is not set. Signed-off-by: Ewan Mellor --- tools/libxc/Makefile | 5 +++++ tools/libxc/xc_bvtsched.c | 8 ++++---- tools/libxc/xc_domain.c | 29 ++++++++++++++--------------- tools/libxc/xc_evtchn.c | 2 +- tools/libxc/xc_gnttab.c | 2 +- tools/libxc/xc_linux_build.c | 7 ++++++- tools/libxc/xc_linux_restore.c | 2 +- tools/libxc/xc_misc.c | 24 ++++++++++++------------ tools/libxc/xc_private.c | 24 ++++++++++++++++-------- tools/libxc/xc_private.h | 17 +++++++++++++++-- tools/libxc/xc_ptrace.c | 4 ++-- tools/libxc/xc_sedf.c | 4 ++-- tools/libxc/xc_tbuf.c | 6 +++--- tools/libxc/xg_private.h | 10 ++++++++++ 14 files changed, 92 insertions(+), 52 deletions(-) diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile index 2f6f581942..08eb0c42a0 100644 --- a/tools/libxc/Makefile +++ b/tools/libxc/Makefile @@ -49,6 +49,11 @@ CFLAGS += -Werror CFLAGS += -O3 CFLAGS += -fno-strict-aliasing CFLAGS += $(INCLUDES) -I. + +# Define this to make it possible to run valgrind on code linked with these +# libraries. +#CFLAGS += -DVALGRIND -O0 -ggdb3 + # Get gcc to generate the dependencies for us. CFLAGS += -Wp,-MD,.$(@F).d LDFLAGS += -L. diff --git a/tools/libxc/xc_bvtsched.c b/tools/libxc/xc_bvtsched.c index fc43383dc4..a7ab88fc4b 100644 --- a/tools/libxc/xc_bvtsched.c +++ b/tools/libxc/xc_bvtsched.c @@ -11,7 +11,7 @@ int xc_bvtsched_global_set(int xc_handle, unsigned long ctx_allow) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_SCHEDCTL; op.u.schedctl.sched_id = SCHED_BVT; @@ -24,7 +24,7 @@ int xc_bvtsched_global_set(int xc_handle, int xc_bvtsched_global_get(int xc_handle, unsigned long *ctx_allow) { - dom0_op_t op; + DECLARE_DOM0_OP; int ret; op.cmd = DOM0_SCHEDCTL; @@ -46,7 +46,7 @@ int xc_bvtsched_domain_set(int xc_handle, long long warpl, long long warpu) { - dom0_op_t op; + DECLARE_DOM0_OP; struct bvt_adjdom *bvtadj = &op.u.adjustdom.u.bvt; op.cmd = DOM0_ADJUSTDOM; @@ -72,7 +72,7 @@ int xc_bvtsched_domain_get(int xc_handle, long long *warpu) { - dom0_op_t op; + DECLARE_DOM0_OP; int ret; struct bvt_adjdom *adjptr = &op.u.adjustdom.u.bvt; diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index e678769cdf..062006094d 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -15,7 +15,7 @@ int xc_domain_create(int xc_handle, uint32_t *pdomid) { int err; - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_CREATEDOMAIN; op.u.createdomain.domain = (domid_t)*pdomid; @@ -32,7 +32,7 @@ int xc_domain_create(int xc_handle, int xc_domain_pause(int xc_handle, uint32_t domid) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_PAUSEDOMAIN; op.u.pausedomain.domain = (domid_t)domid; return do_dom0_op(xc_handle, &op); @@ -42,7 +42,7 @@ int xc_domain_pause(int xc_handle, int xc_domain_unpause(int xc_handle, uint32_t domid) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_UNPAUSEDOMAIN; op.u.unpausedomain.domain = (domid_t)domid; return do_dom0_op(xc_handle, &op); @@ -52,7 +52,7 @@ int xc_domain_unpause(int xc_handle, int xc_domain_destroy(int xc_handle, uint32_t domid) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_DESTROYDOMAIN; op.u.destroydomain.domain = (domid_t)domid; return do_dom0_op(xc_handle, &op); @@ -63,7 +63,7 @@ int xc_domain_pincpu(int xc_handle, int vcpu, cpumap_t cpumap) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_PINCPUDOMAIN; op.u.pincpudomain.domain = (domid_t)domid; op.u.pincpudomain.vcpu = vcpu; @@ -79,7 +79,7 @@ int xc_domain_getinfo(int xc_handle, { unsigned int nr_doms; uint32_t next_domid = first_domid; - dom0_op_t op; + DECLARE_DOM0_OP; int rc = 0; memset(info, 0, max_doms*sizeof(xc_dominfo_t)); @@ -134,7 +134,7 @@ int xc_domain_getinfolist(int xc_handle, xc_domaininfo_t *info) { int ret = 0; - dom0_op_t op; + DECLARE_DOM0_OP; if ( mlock(info, max_domains*sizeof(xc_domaininfo_t)) != 0 ) return -1; @@ -161,7 +161,7 @@ int xc_domain_get_vcpu_context(int xc_handle, vcpu_guest_context_t *ctxt) { int rc; - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_GETVCPUCONTEXT; op.u.getvcpucontext.domain = (domid_t)domid; @@ -187,7 +187,7 @@ int xc_shadow_control(int xc_handle, xc_shadow_control_stats_t *stats ) { int rc; - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_SHADOW_CONTROL; op.u.shadow_control.domain = (domid_t)domid; op.u.shadow_control.op = sop; @@ -250,7 +250,7 @@ int xc_domain_setmaxmem(int xc_handle, uint32_t domid, unsigned int max_memkb) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_SETDOMAINMAXMEM; op.u.setdomainmaxmem.domain = (domid_t)domid; op.u.setdomainmaxmem.max_memkb = max_memkb; @@ -328,7 +328,7 @@ int xc_domain_memory_decrease_reservation(int xc_handle, int xc_domain_max_vcpus(int xc_handle, uint32_t domid, unsigned int max) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_MAX_VCPUS; op.u.max_vcpus.domain = (domid_t)domid; op.u.max_vcpus.max = max; @@ -338,7 +338,7 @@ int xc_domain_max_vcpus(int xc_handle, uint32_t domid, unsigned int max) int xc_domain_sethandle(int xc_handle, uint32_t domid, xen_domain_handle_t handle) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_SETDOMAINHANDLE; op.u.setdomainhandle.domain = (domid_t)domid; memcpy(op.u.setdomainhandle.handle, handle, sizeof(xen_domain_handle_t)); @@ -351,8 +351,7 @@ int xc_domain_get_vcpu_info(int xc_handle, xc_vcpuinfo_t *info) { int rc; - dom0_op_t op; - + DECLARE_DOM0_OP; op.cmd = DOM0_GETVCPUINFO; op.u.getvcpuinfo.domain = (domid_t)domid; op.u.getvcpuinfo.vcpu = (uint16_t)vcpu; @@ -370,7 +369,7 @@ int xc_domain_ioport_permission(int xc_handle, uint16_t nr_ports, uint16_t allow_access) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_IOPORT_PERMISSION; op.u.ioport_permission.domain = (domid_t)domid; diff --git a/tools/libxc/xc_evtchn.c b/tools/libxc/xc_evtchn.c index af79ef384a..ffc252ed5a 100644 --- a/tools/libxc/xc_evtchn.c +++ b/tools/libxc/xc_evtchn.c @@ -12,7 +12,7 @@ static int do_evtchn_op(int xc_handle, evtchn_op_t *op) { int ret = -1; - privcmd_hypercall_t hypercall; + DECLARE_HYPERCALL; hypercall.op = __HYPERVISOR_event_channel_op; hypercall.arg[0] = (unsigned long)op; diff --git a/tools/libxc/xc_gnttab.c b/tools/libxc/xc_gnttab.c index fd9df4c1b2..54fb40e6e1 100644 --- a/tools/libxc/xc_gnttab.c +++ b/tools/libxc/xc_gnttab.c @@ -17,7 +17,7 @@ do_gnttab_op(int xc_handle, unsigned long count) { int ret = -1; - privcmd_hypercall_t hypercall; + DECLARE_HYPERCALL; hypercall.op = __HYPERVISOR_grant_table_op; hypercall.arg[0] = cmd; diff --git a/tools/libxc/xc_linux_build.c b/tools/libxc/xc_linux_build.c index cb72db04d6..d61ecfa549 100644 --- a/tools/libxc/xc_linux_build.c +++ b/tools/libxc/xc_linux_build.c @@ -692,7 +692,8 @@ int xc_linux_build(int xc_handle, unsigned int console_evtchn, unsigned long *console_mfn) { - dom0_op_t launch_op, op; + dom0_op_t launch_op; + DECLARE_DOM0_OP; int initrd_fd = -1; gzFile initrd_gfd = NULL; int rc, i; @@ -728,6 +729,10 @@ int xc_linux_build(int xc_handle, } } +#ifdef VALGRIND + memset(&st_ctxt, 0, sizeof(st_ctxt)); +#endif + if ( mlock(&st_ctxt, sizeof(st_ctxt) ) ) { PERROR("%s: ctxt mlock failed", __func__); diff --git a/tools/libxc/xc_linux_restore.c b/tools/libxc/xc_linux_restore.c index 88c09c797c..20dfde7d98 100644 --- a/tools/libxc/xc_linux_restore.c +++ b/tools/libxc/xc_linux_restore.c @@ -106,7 +106,7 @@ int xc_linux_restore(int xc_handle, int io_fd, unsigned int store_evtchn, unsigned long *store_mfn, unsigned int console_evtchn, unsigned long *console_mfn) { - dom0_op_t op; + DECLARE_DOM0_OP; int rc = 1, i, n; unsigned long mfn, pfn; unsigned int prev_pc, this_pc; diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c index b0987d2a47..13848fbf5f 100644 --- a/tools/libxc/xc_misc.c +++ b/tools/libxc/xc_misc.c @@ -25,7 +25,7 @@ int xc_readconsolering(int xc_handle, int clear) { int ret; - dom0_op_t op; + DECLARE_DOM0_OP; char *buffer = *pbuffer; unsigned int nr_chars = *pnr_chars; @@ -52,7 +52,7 @@ int xc_physinfo(int xc_handle, xc_physinfo_t *put_info) { int ret; - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_PHYSINFO; op.interface_version = DOM0_INTERFACE_VERSION; @@ -69,7 +69,7 @@ int xc_sched_id(int xc_handle, int *sched_id) { int ret; - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_SCHED_ID; op.interface_version = DOM0_INTERFACE_VERSION; @@ -83,25 +83,25 @@ int xc_sched_id(int xc_handle, } int xc_perfc_control(int xc_handle, - uint32_t op, + uint32_t opcode, xc_perfc_desc_t *desc) { int rc; - dom0_op_t dop; + DECLARE_DOM0_OP; - dop.cmd = DOM0_PERFCCONTROL; - dop.u.perfccontrol.op = op; - dop.u.perfccontrol.desc = desc; + op.cmd = DOM0_PERFCCONTROL; + op.u.perfccontrol.op = opcode; + op.u.perfccontrol.desc = desc; - rc = do_dom0_op(xc_handle, &dop); + rc = do_dom0_op(xc_handle, &op); - return (rc == 0) ? dop.u.perfccontrol.nr_counters : rc; + return (rc == 0) ? op.u.perfccontrol.nr_counters : rc; } long long xc_msr_read(int xc_handle, int cpu_mask, int msr) { int rc; - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_MSR; op.u.msr.write = 0; @@ -117,7 +117,7 @@ int xc_msr_write(int xc_handle, int cpu_mask, int msr, unsigned int low, unsigned int high) { int rc; - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_MSR; op.u.msr.write = 1; diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c index 526d77b337..2c7759336e 100644 --- a/tools/libxc/xc_private.c +++ b/tools/libxc/xc_private.c @@ -68,7 +68,7 @@ void *xc_map_foreign_range(int xc_handle, uint32_t dom, int xc_get_pfn_type_batch(int xc_handle, uint32_t dom, int num, unsigned long *arr) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_GETPAGEFRAMEINFO2; op.u.getpageframeinfo2.domain = (domid_t)dom; op.u.getpageframeinfo2.num = num; @@ -81,7 +81,7 @@ unsigned int get_pfn_type(int xc_handle, unsigned long mfn, uint32_t dom) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_GETPAGEFRAMEINFO; op.u.getpageframeinfo.pfn = mfn; op.u.getpageframeinfo.domain = (domid_t)dom; @@ -99,7 +99,7 @@ int xc_mmuext_op( unsigned int nr_ops, domid_t dom) { - privcmd_hypercall_t hypercall; + DECLARE_HYPERCALL; long ret = -EINVAL; hypercall.op = __HYPERVISOR_mmuext_op; @@ -125,7 +125,7 @@ int xc_mmuext_op( static int flush_mmu_updates(int xc_handle, xc_mmu_t *mmu) { int err = 0; - privcmd_hypercall_t hypercall; + DECLARE_HYPERCALL; if ( mmu->idx == 0 ) return 0; @@ -188,7 +188,7 @@ int xc_memory_op(int xc_handle, int cmd, void *arg) { - privcmd_hypercall_t hypercall; + DECLARE_HYPERCALL; struct xen_memory_reservation *reservation = arg; long ret = -EINVAL; @@ -236,7 +236,7 @@ int xc_memory_op(int xc_handle, long long xc_domain_get_cpu_usage( int xc_handle, domid_t domid, int vcpu ) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_GETVCPUINFO; op.u.getvcpuinfo.domain = (domid_t)domid; @@ -255,13 +255,16 @@ int xc_get_pfn_list(int xc_handle, unsigned long *pfn_buf, unsigned long max_pfns) { - dom0_op_t op; + DECLARE_DOM0_OP; int ret; op.cmd = DOM0_GETMEMLIST; op.u.getmemlist.domain = (domid_t)domid; op.u.getmemlist.max_pfns = max_pfns; op.u.getmemlist.buffer = pfn_buf; +#ifdef VALGRIND + memset(pfn_buf, 0, max_pfns * sizeof(unsigned long)); +#endif if ( mlock(pfn_buf, max_pfns * sizeof(unsigned long)) != 0 ) { @@ -293,7 +296,7 @@ int xc_get_pfn_list(int xc_handle, long xc_get_tot_pages(int xc_handle, uint32_t domid) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_GETDOMAININFO; op.u.getdomaininfo.domain = (domid_t)domid; return (do_dom0_op(xc_handle, &op) < 0) ? @@ -403,6 +406,11 @@ int xc_version(int xc_handle, int cmd, void *arg) return -ENOMEM; } +#ifdef VALGRIND + if (argsize != 0) + memset(arg, 0, argsize); +#endif + rc = do_xen_version(xc_handle, cmd, arg); if ( argsize != 0 ) diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h index 0e20d2db59..2a3164c1ac 100644 --- a/tools/libxc/xc_private.h +++ b/tools/libxc/xc_private.h @@ -17,6 +17,19 @@ #include +/* valgrind cannot see when a hypercall has filled in some values. For this + reason, we must zero the privcmd_hypercall_t or dom0_op_t instance before a + call, if using valgrind. */ +#ifdef VALGRIND +#define DECLARE_HYPERCALL privcmd_hypercall_t hypercall; \ + memset(&hypercall, 0, sizeof(hypercall)) +#define DECLARE_DOM0_OP dom0_op_t op; memset(&op, 0, sizeof(op)) +#else +#define DECLARE_HYPERCALL privcmd_hypercall_t hypercall +#define DECLARE_DOM0_OP dom0_op_t op +#endif + + #define PAGE_SHIFT XC_PAGE_SHIFT #define PAGE_SIZE (1UL << PAGE_SHIFT) #define PAGE_MASK (~(PAGE_SIZE-1)) @@ -61,7 +74,7 @@ static inline int do_xen_hypercall(int xc_handle, static inline int do_xen_version(int xc_handle, int cmd, void *dest) { - privcmd_hypercall_t hypercall; + DECLARE_HYPERCALL; hypercall.op = __HYPERVISOR_xen_version; hypercall.arg[0] = (unsigned long) cmd; @@ -73,7 +86,7 @@ static inline int do_xen_version(int xc_handle, int cmd, void *dest) static inline int do_dom0_op(int xc_handle, dom0_op_t *op) { int ret = -1; - privcmd_hypercall_t hypercall; + DECLARE_HYPERCALL; op->interface_version = DOM0_INTERFACE_VERSION; diff --git a/tools/libxc/xc_ptrace.c b/tools/libxc/xc_ptrace.c index 5bddbbff1f..c101d3225b 100644 --- a/tools/libxc/xc_ptrace.c +++ b/tools/libxc/xc_ptrace.c @@ -283,7 +283,7 @@ xc_waitdomain( int *status, int options) { - dom0_op_t op; + DECLARE_DOM0_OP; int retval; struct timespec ts; ts.tv_sec = 0; @@ -323,7 +323,7 @@ xc_ptrace( long eaddr, long edata) { - dom0_op_t op; + DECLARE_DOM0_OP; int status = 0; struct gdb_regs pt; long retval = 0; diff --git a/tools/libxc/xc_sedf.c b/tools/libxc/xc_sedf.c index 64984afa12..34216ed97e 100644 --- a/tools/libxc/xc_sedf.c +++ b/tools/libxc/xc_sedf.c @@ -13,7 +13,7 @@ int xc_sedf_domain_set(int xc_handle, uint32_t domid, uint64_t period, uint64_t slice,uint64_t latency, uint16_t extratime,uint16_t weight) { - dom0_op_t op; + DECLARE_DOM0_OP; struct sedf_adjdom *p = &op.u.adjustdom.u.sedf; op.cmd = DOM0_ADJUSTDOM; @@ -31,7 +31,7 @@ int xc_sedf_domain_set(int xc_handle, int xc_sedf_domain_get(int xc_handle, uint32_t domid, uint64_t *period, uint64_t *slice, uint64_t* latency, uint16_t* extratime, uint16_t* weight) { - dom0_op_t op; + DECLARE_DOM0_OP; int ret; struct sedf_adjdom *p = &op.u.adjustdom.u.sedf; diff --git a/tools/libxc/xc_tbuf.c b/tools/libxc/xc_tbuf.c index b47cde2867..b9a5e51a01 100644 --- a/tools/libxc/xc_tbuf.c +++ b/tools/libxc/xc_tbuf.c @@ -10,7 +10,7 @@ int xc_tbuf_enable(int xc_handle, int enable) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_TBUFCONTROL; op.interface_version = DOM0_INTERFACE_VERSION; @@ -24,7 +24,7 @@ int xc_tbuf_enable(int xc_handle, int enable) int xc_tbuf_set_size(int xc_handle, uint32_t size) { - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_TBUFCONTROL; op.interface_version = DOM0_INTERFACE_VERSION; @@ -37,7 +37,7 @@ int xc_tbuf_set_size(int xc_handle, uint32_t size) int xc_tbuf_get_size(int xc_handle, uint32_t *size) { int rc; - dom0_op_t op; + DECLARE_DOM0_OP; op.cmd = DOM0_TBUFCONTROL; op.interface_version = DOM0_INTERFACE_VERSION; diff --git a/tools/libxc/xg_private.h b/tools/libxc/xg_private.h index ab0b188ffc..a2dac8b0f3 100644 --- a/tools/libxc/xg_private.h +++ b/tools/libxc/xg_private.h @@ -16,6 +16,16 @@ #include #include +/* valgrind cannot see when a hypercall has filled in some values. For this + reason, we must zero the dom0_op_t instance before a call, if using + valgrind. */ +#ifdef VALGRIND +#define DECLARE_DOM0_OP dom0_op_t op; memset(&op, 0, sizeof(op)) +#else +#define DECLARE_DOM0_OP dom0_op_t op +#endif + + char *xc_read_kernel_image(const char *filename, unsigned long *size); unsigned long csum_page (void * page); -- 2.30.2